Skip to content

Remove stale Coveralls integration#3204

Merged
justin808 merged 6 commits intomainfrom
jg-conductor/issue-1692-badge
Apr 29, 2026
Merged

Remove stale Coveralls integration#3204
justin808 merged 6 commits intomainfrom
jg-conductor/issue-1692-badge

Conversation

@justin808
Copy link
Copy Markdown
Member

@justin808 justin808 commented Apr 24, 2026

Summary

  • remove the stale Coveralls badge and the dead Coveralls-only rake paths
  • switch repo and generator coverage support to direct simplecov dependencies
  • update the coverage docs and replace the broken README Slack invite URL so branch checks pass

Closes #1692.

Testing

  • bundle exec rubocop react_on_rails/Rakefile react_on_rails/Gemfile.development_dependencies react_on_rails/lib/generators/react_on_rails/dev_tests_generator.rb react_on_rails/rakelib/run_rspec.rake react_on_rails/rakelib/shakapacker_examples.rake react_on_rails/spec/react_on_rails/generators/dev_tests_generator_spec.rb react_on_rails_pro/Rakefile react_on_rails_pro/Gemfile.development_dependencies react_on_rails_pro/rakelib/run_rspec.rake
  • bundle exec rspec react_on_rails/spec/react_on_rails/generators/dev_tests_generator_spec.rb
  • bundle exec rake -T run_rspec
  • (cd react_on_rails_pro && bundle exec rake -T run_rspec)
  • bundle exec rspec react_on_rails/spec/empty_spec.rb
  • (cd react_on_rails_pro && bundle exec rspec spec/empty_spec.rb)

Note

Low Risk
Low risk since changes are limited to test/CI tooling and documentation, but could affect coverage reporting if any workflows still expect Coveralls tasks or binstubs.

Overview
Removes the stale Coveralls integration across both OSS and Pro: drops the Coveralls badge, deletes Coveralls-only rake hooks/tasks, and removes Coveralls binstubs from dummy apps.

Switches coverage support to direct simplecov dependencies (including the dev_tests generator and lockfiles) and updates docs to clarify that coverage reports are produced when running tests with COVERAGE=true. Also updates the React+Rails Slack link to the direct workspace URL.

Reviewed by Cursor Bugbot for commit 80c86a8. Bugbot is set up for automated code reviews on this repo. Configure here.

Summary by CodeRabbit

  • Chores
    • Switched test coverage tooling from Coveralls to SimpleCov and removed Coveralls integration/hooks.
  • Documentation
    • Clarified coverage reports are shown only when running tests with COVERAGE=true.
    • Removed the Coverage Status badge and updated Slack support links to the direct workspace URL in README and docs.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 24, 2026

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 08926748-823f-4a5e-9f70-e8d66aae3d3d

📥 Commits

Reviewing files that changed from the base of the PR and between 80c86a8 and 42d2e0c.

⛔ Files ignored due to path filters (4)
  • react_on_rails/Gemfile.lock is excluded by !**/*.lock
  • react_on_rails/spec/dummy/Gemfile.lock is excluded by !**/*.lock
  • react_on_rails_pro/Gemfile.lock is excluded by !**/*.lock
  • react_on_rails_pro/spec/dummy/Gemfile.lock is excluded by !**/*.lock
📒 Files selected for processing (4)
  • README.md
  • react_on_rails/Gemfile.development_dependencies
  • react_on_rails_pro/Gemfile.development_dependencies
  • react_on_rails_pro/rakelib/run_rspec.rake
✅ Files skipped from review due to trivial changes (1)
  • react_on_rails/Gemfile.development_dependencies
🚧 Files skipped from review as they are similar to previous changes (2)
  • README.md
  • react_on_rails_pro/Gemfile.development_dependencies

Walkthrough

Removes Coveralls integration and replaces it with SimpleCov across dev/test tooling, generators, Rake tasks, dummy binstubs, tests, and documentation; docs now require COVERAGE=true to generate SimpleCov reports and Slack links updated to the workspace URL. (49 words)

Changes

Cohort / File(s) Summary
Documentation & Help Links
CONTRIBUTING.md, react_on_rails_pro/CONTRIBUTING.md, docs/oss/deployment/troubleshooting.md, docs/oss/getting-started/common-issues.md
Clarified that SimpleCov reports are produced only when tests run with COVERAGE=true; updated Slack links from invite URL to direct workspace URL and added membership guidance.
README Badge & Links
README.md
Removed Coverage Status badge from top badge cluster and changed the React + Rails Slack link to the workspace URL with parenthetical membership guidance.
Test Coverage Dependencies
react_on_rails/Gemfile.development_dependencies, react_on_rails_pro/Gemfile.development_dependencies
Replaced coveralls with simplecov (require: false, group: :test); pro Gemfile also normalized quoting to double quotes.
Rake & CI Integration
react_on_rails/Rakefile, react_on_rails_pro/Rakefile, react_on_rails/rakelib/run_rspec.rake, react_on_rails_pro/rakelib/run_rspec.rake
Removed Coveralls-specific requires, the Coveralls::RakeTask registration, and the USE_COVERALLS-driven injection of COVERAGE=true; adjusted run/skip behavior to require ENV["COVERAGE"] == "true" for generating coverage.
Generators & Specs
react_on_rails/lib/generators/react_on_rails/dev_tests_generator.rb, react_on_rails/spec/react_on_rails/generators/dev_tests_generator_spec.rb
Generator now adds simplecov (require: false, group: :test) instead of coveralls; test expectation updated to match.
Deleted Dummy Binstubs
react_on_rails/spec/dummy/bin/coveralls, react_on_rails_pro/spec/dummy/bin/coveralls
Removed dummy app Coveralls CLI wrapper scripts; no replacement provided.
Rakefile Comments
react_on_rails/rakelib/shakapacker_examples.rake
Updated inline comment to reference simplecov instead of coveralls.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

Poem

🐰 I swapped the clouds for local light,
SimpleCov hops in, cozy and bright.
Run your specs with COVERAGE=true,
Reports appear in coverage—fresh view.
A tiny thump for tidy links and hue.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately and concisely summarizes the main objective: removing the outdated Coveralls integration that was tied to deprecated CI.
Linked Issues check ✅ Passed The PR fully addresses issue #1692 requirements: removes the stale Coveralls README badge, eliminates USE_COVERALLS-gated Rake tasks, deletes coveralls binstubs, and updates documentation.
Out of Scope Changes check ✅ Passed Minor documentation changes to Slack links and SimpleCov setup are reasonable supporting changes but slightly tangential. However, they align with modernizing community links and ensuring coverage tooling works properly.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch jg-conductor/issue-1692-badge

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share
Review rate limit: 7/8 reviews remaining, refill in 7 minutes and 30 seconds.

Comment @coderabbitai help to get the list of available commands and usage tips.

@greptile-apps
Copy link
Copy Markdown

greptile-apps Bot commented Apr 24, 2026

Greptile Summary

This PR removes the stale Coveralls integration across both react_on_rails and react_on_rails_pro, replacing it with a direct simplecov dependency and making coverage opt-in via COVERAGE=true. It also drops the dead bin/coveralls binstubs, removes the broken Coveralls README badge, fixes the Slack invite URL, and updates documentation accordingly.

Confidence Score: 5/5

Safe to merge — purely removes dead code and stale dependencies with no functional regressions.

All changes are straightforward removals of an unmaintained integration. The only finding is a P2 description mismatch on the run_rspec:empty rake task, which does not affect runtime behaviour.

react_on_rails_pro/rakelib/run_rspec.rake — minor task description mismatch after removing the automatic COVERAGE=true flag.

Important Files Changed

Filename Overview
react_on_rails/Rakefile Removes the USE_COVERALLS-gated coveralls:push task; clean removal with no regressions.
react_on_rails/rakelib/run_rspec.rake Removes all USE_COVERALLS guards and the automatic COVERAGE=true injection; straightforward cleanup.
react_on_rails_pro/rakelib/run_rspec.rake Removes coveralls rake integration; run_rspec:empty task description no longer reflects actual behaviour since COVERAGE=true is not set.
react_on_rails/lib/generators/react_on_rails/dev_tests_generator.rb Swaps coveralls for simplecov in the generated Gemfile snippet; generator test updated to match.
react_on_rails/Gemfile.development_dependencies Replaces coveralls with simplecov (no version pin); Gemfile.lock updated consistently.
react_on_rails_pro/Gemfile.development_dependencies Replaces coveralls with simplecov and normalises single-quoted strings to double-quoted throughout.
README.md Removes stale Coveralls badge and updates the Slack community link from the broken invite URL to the workspace URL.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[rake default / rake run_rspec] --> B[run_rspec tasks]
    B --> C[run_rspec:gem]
    B --> D[run_rspec:dummy]
    B --> E[run_rspec:empty]
    B --> F[run_rspec:js_tests]

    C --> G["bundle exec rspec (gem specs)"]
    D --> H["bundle exec rspec (dummy app)"]
    E --> I["rspec spec/empty_spec.rb\n(no COVERAGE=true)"]
    F --> J["pnpm run test"]

    K["COVERAGE=true\n(user-set)"] -.->|"opt-in"| G
    K -.->|"opt-in"| H

    style I fill:#fff3cd,stroke:#ffc107
Loading

Reviews (1): Last reviewed commit: "Use a working Slack URL in the README" | Re-trigger Greptile

Comment thread react_on_rails_pro/rakelib/run_rspec.rake Outdated
@claude
Copy link
Copy Markdown
Contributor

claude Bot commented Apr 24, 2026

Code Review — PR #3204: Remove stale Coveralls integration

Overview

Clean, well-scoped removal of the unmaintained coveralls gem (v0.8.23, last updated ~2019) across both OSS and Pro. The simplecov dependency it was hiding is now explicit. Coverage behavior is preserved via COVERAGE=true env inheritance to child processes, and the CONTRIBUTING docs are updated to document this.


What works well

  • Complete removal — Coveralls binstubs, Rake tasks, USE_COVERALLS guards, and lock file entries are all gone consistently across OSS and Pro
  • empty_spec.rb task is correct — The Pro run_rspec:empty task previously prepended COVERAGE=true only when USE_COVERALLS was set. Since empty_spec.rb unconditionally require "simplecov" (it just triggers cache-based report generation), removing that prepend is the right call
  • Env inheritance works — Running COVERAGE=true bundle exec rake run_rspec still propagates COVERAGE=true to child processes via sh, so no explicit token injection is needed
  • Generator template is already SimpleCov-nativesimplecov_helper.rb in the template already gates on ENV["COVERAGE"] == "true", so generated apps work correctly

Issues / suggestions

1. SimpleCov still locked to v0.16.1 in lock files

coveralls constrained simplecov (~> 0.16.1), which is why both lock files resolved to simplecov (0.16.1) (released 2019). Now that simplecov is an unconstrained direct dependency, the current lock files stay on the old version until someone explicitly runs bundle update simplecov. This PR is a good opportunity to pick up ~4 years of improvements (better HTML reports, parallel test merging, etc.). Not a blocker, but worth a follow-up or doing now.

2. Slack community URL — invite link vs. workspace link

Changing invite.reactrails.comreactrails.slack.com: if the invite link was dead/broken, this fix is correct. But the direct workspace URL requires an existing workspace member account to be useful — new community members cannot join without a valid invite link. Please verify whether invite.reactrails.com is genuinely broken before merging, or whether there is an updated invite URL available. If no invite link exists, the current change is acceptable; just note that the README will no longer let new members self-join the Slack.

3. simplecov in generator Gemfile has no group: :test

gem("simplecov", require: false) in dev_tests_generator.rb adds the gem to all groups. This matches the old coveralls behavior (not a regression), but since rspec-rails in the same method uses group: :test, aligning simplecov the same way would be cleaner. Minor.


Verdict

Approve with minor notes. The removal is correct and complete. The Slack URL change is the only item worth confirming before merge.

Comment thread react_on_rails/lib/generators/react_on_rails/dev_tests_generator.rb Outdated
Comment thread README.md
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@react_on_rails_pro/rakelib/run_rspec.rake`:
- Line 27: Replace the bare rspec invocation in the sh call so it uses the
Bundler environment; specifically update the command string "sh \"rspec
spec/empty_spec.rb\"" to use "sh \"bundle exec rspec spec/empty_spec.rb\"" so
the RSpec run in run_rspec.rake (the sh call that runs spec/empty_spec.rb) uses
the project's Gemfile and matches other invocations like run_tests_in.

In `@README.md`:
- Line 118: The README currently links "React + Rails Slack" to the workspace
URL that blocks new signups; replace the URL for that link with the public
invite URL provided
(https://join.slack.com/t/reactrails/shared_invite/zt-38oicm9d0-OO0V~bdg4aYNuZuUbRFSXg)
so the "React + Rails Slack" anchor allows new users to join and create accounts
during onboarding.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: e4b044a0-7602-4a22-a034-82e6df3d16ec

📥 Commits

Reviewing files that changed from the base of the PR and between 3605761 and 7dee749.

⛔ Files ignored due to path filters (4)
  • react_on_rails/Gemfile.lock is excluded by !**/*.lock
  • react_on_rails/spec/dummy/Gemfile.lock is excluded by !**/*.lock
  • react_on_rails_pro/Gemfile.lock is excluded by !**/*.lock
  • react_on_rails_pro/spec/dummy/Gemfile.lock is excluded by !**/*.lock
📒 Files selected for processing (14)
  • CONTRIBUTING.md
  • README.md
  • react_on_rails/Gemfile.development_dependencies
  • react_on_rails/Rakefile
  • react_on_rails/lib/generators/react_on_rails/dev_tests_generator.rb
  • react_on_rails/rakelib/run_rspec.rake
  • react_on_rails/rakelib/shakapacker_examples.rake
  • react_on_rails/spec/dummy/bin/coveralls
  • react_on_rails/spec/react_on_rails/generators/dev_tests_generator_spec.rb
  • react_on_rails_pro/CONTRIBUTING.md
  • react_on_rails_pro/Gemfile.development_dependencies
  • react_on_rails_pro/Rakefile
  • react_on_rails_pro/rakelib/run_rspec.rake
  • react_on_rails_pro/spec/dummy/bin/coveralls
💤 Files with no reviewable changes (5)
  • react_on_rails_pro/spec/dummy/bin/coveralls
  • react_on_rails_pro/Rakefile
  • react_on_rails/Rakefile
  • react_on_rails/rakelib/run_rspec.rake
  • react_on_rails/spec/dummy/bin/coveralls

Comment thread react_on_rails_pro/rakelib/run_rspec.rake Outdated
Comment thread README.md
@claude
Copy link
Copy Markdown
Contributor

claude Bot commented Apr 25, 2026

Code Review – Remove stale Coveralls integration (#3204)

Overview

Clean dependency-cleanup PR. Coveralls 0.8.23 was effectively abandoned and the whole integration was dead (badge pointed at an old Travis run). The PR correctly removes every Coveralls artifact – rake tasks, binstubs, gem entries – and registers a direct simplecov dependency instead. The COVERAGE=true gating was already in place in the simplecov_helper.rb files; this PR just aligns the docs with that reality.

What works well

  • All Coveralls references removed consistently across OSS + Pro (Gemfile, Gemfile.lock, Rakefiles, binstubs, CI rake paths).
  • Generator and its spec stay in sync: dev_tests_generator.rb now emits simplecov; the spec asserts exactly that.
  • The empty task simplification is correct – empty_spec.rb unconditionally require "simplecov", so it never needed COVERAGE=true forwarded through the env.
  • Bonus quote-style normalisation in react_on_rails_pro/Gemfile.development_dependencies is harmless and improves consistency.

Issues / suggestions

1. Slack URL may not be a self-join link
https://reactrails.slack.com opens the workspace home page rather than an invite link. New users who are not already members will hit a "You need an invitation" wall. The old https://invite.reactrails.com redirect presumably served a real invite URL. Worth verifying that there is a public invite link and using it (or adding a note to the docs that you need to request an invite first).

2. Trailing blank line after deleted content in react_on_rails_pro/rakelib/run_rspec.rake
After removing the require "coveralls/rake/task" line there is an orphaned blank line directly after the # TODO: This file is not used for CI comment, giving the file two consecutive blank lines at the top. Minor, but worth squashing.

3. No version constraint on the new simplecov dependency
The old coveralls gem pinned simplecov (~> 0.16.1). The direct dep added here has no constraint, so bundle update simplecov can now pick up any future major. That is generally desirable, but if anything in the codebase relies on 0.16.x APIs the bump could be silent. A quick pass over the SimpleCov.start blocks shows only standard APIs (command_name, add_filter, start("rails")), so this is low risk – just worth being aware of.

4. One historical mention of Coveralls remains
analysis/open-issue-deep-dive/wave-3-long-tail.md:116 still references the old Coveralls issue. It is a historical analysis document so leaving it is fine, but if the goal is zero references across the repo it could be noted.

Verdict

Approve with minor comments. The Slack URL is the only item worth a second look before merge; everything else is low-stakes.

Comment thread README.md
Comment thread react_on_rails_pro/rakelib/run_rspec.rake
Comment thread react_on_rails/Gemfile.development_dependencies Outdated
@justin808
Copy link
Copy Markdown
Member Author

Follow-up on bot feedback:

  • fixed react_on_rails_pro/rakelib/run_rspec.rake so run_rspec:empty now uses bundle exec rspec spec/empty_spec.rb
  • did not switch the Slack links to the suggested public invite URL because that exact URL currently returns 403 when verified from this branch (curl -I -L) and would reintroduce a broken markdown link

If there is a newer public invite or a stable landing page for Slack onboarding, I can switch the docs to that in a follow-up.

@claude
Copy link
Copy Markdown
Contributor

claude Bot commented Apr 25, 2026

Code Review

Overview

This PR cleanly removes the stale Coveralls integration, replacing it with direct simplecov dependencies in both OSS and Pro packages. The changes are low-risk and well-scoped. The Coveralls gem was pinned to an old simplecov (~> 0.16.1) constraint, so this is also a dependency hygiene improvement.


What's Good

  • Complete removal of USE_COVERALLS-gated code paths across Rakefiles, rake tasks, binstubs, and Gemfiles — no half-measures.
  • Quote style cleanup in react_on_rails_pro/Gemfile.development_dependencies (single → double quotes) is a welcome consistency fix.
  • CONTRIBUTING.md and react_on_rails_pro/CONTRIBUTING.md docs are updated to accurately reflect the COVERAGE=true requirement.
  • CI workflow files have no remaining coveralls or USE_COVERALLS references — clean cut.

Issues Found

Bug: simplecov added to Gemfile without group: :test in the generator

In dev_tests_generator.rb, rspec-rails is correctly scoped to :test, but simplecov is not:

gem("rspec-rails", group: :test)
gem("simplecov", require: false)   # ← missing group: :test

When the dev_tests generator runs on a user's app this adds SimpleCov to the global Gemfile scope, not the test group. Since SimpleCov is exclusively a test-time concern this should mirror the rspec-rails call. The generator spec also doesn't assert the group is present, so it wouldn't catch this.


Minor: Slack workspace URL doesn't let new users join

The PR replaces https://invite.reactrails.com (broken invite link) with https://reactrails.slack.com (direct workspace URL). A direct workspace URL requires the visitor to already have access — it doesn't present a join/invite flow. If the Slack workspace is invite-only, new community members can't join from this link. Consider using Slack's shared-invite mechanism or noting in the docs that an invite is needed.


Minor: no version constraint on simplecov

The old coveralls (0.8.23) gem depended on simplecov (~> 0.16.1), which acted as an implicit pin. The new direct gem "simplecov" entry has no constraint, so bundle update simplecov could pull in any newer version. Both lock files currently resolve to 0.16.1, so there's no immediate risk, but a constraint like "~> 0.16" would preserve the existing behavior deliberately rather than by accident.

Comment thread react_on_rails/lib/generators/react_on_rails/dev_tests_generator.rb Outdated
Comment thread react_on_rails/spec/react_on_rails/generators/dev_tests_generator_spec.rb Outdated
- Scope generated simplecov gem to the test group (matches the
  rspec-rails entry directly above and the dev Gemfiles)
- Tighten the dev_tests generator spec to assert the new group
- Clarify the Pro run_rspec:empty task description so it's obvious
  that COVERAGE=true is opt-in

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@justin808
Copy link
Copy Markdown
Member Author

Review triage summary (commit 80c86a8)

I (Claude Code) walked the bot review feedback and split it into MUST-FIX, OPTIONAL, and SKIPPED. Code changes for the first two landed in 80c86a8; SKIPPED items got rationale replies on their threads and the resolved threads have been closed.

Addressed in code (80c86a8)

  • react_on_rails/lib/generators/react_on_rails/dev_tests_generator.rb:44 — added group: :test to the generated simplecov line so it matches the rspec-rails entry directly above and stops leaking into the global Gemfile of generated apps.
  • react_on_rails/spec/react_on_rails/generators/dev_tests_generator_spec.rb:52 — tightened the assertion to require group: :test so a future regression is caught.
  • react_on_rails_pro/rakelib/run_rspec.rake:25 — reworded the run_rspec:empty desc to make it explicit that COVERAGE=true is opt-in.

Declined / non-actionable (replied with rationale, threads resolved)

  • README Slack URL (3 threads, claude[bot] + coderabbitai[bot]) — the suggested public invite URL returns 403 (already verified by the PR author in Remove stale Coveralls integration #3204 (comment)). Workspace URL stays until a working invite landing page exists.
  • react_on_rails_pro/rakelib/run_rspec.rake:5 "two consecutive blank lines" — verified the file actually only has one blank line between the # TODO comment and require "pathname".
  • react_on_rails/Gemfile.development_dependencies:51 "no version pin on simplecov" — informational; leaving simplecov unconstrained intentionally so this PR isn't also a SimpleCov bump.

Verification: bundle exec rspec spec/react_on_rails/generators/dev_tests_generator_spec.rb passes (6 examples, 0 failures); bundle exec rubocop is clean on all three changed files (pre-commit hook also ran cleanly).

Comment thread README.md
Comment thread react_on_rails_pro/rakelib/run_rspec.rake Outdated
@claude
Copy link
Copy Markdown
Contributor

claude Bot commented Apr 25, 2026

Review: Remove stale Coveralls integration

Overall: Clean, low-risk cleanup — approve with two minor points.

What this does well

  • Removes the long-stale Coveralls 0.8.23 gem and all USE_COVERALLS-gated Rake hooks. Coveralls had been effectively unmaintained for years and was the only thing holding simplecov at ~> 0.16.1 (currently locked at 0.16.1 despite 0.22.x being available).
  • Promotes simplecov from a transitive dep to a direct one — honest about what the project actually uses.
  • Deletes the generated bin/coveralls binstubs from both dummy apps correctly.
  • dev_tests_generator now injects simplecov instead of coveralls, and the generator spec is updated to match — consistent end-to-end.
  • Normalises single → double quotes in react_on_rails_pro/Gemfile.development_dependencies to match project convention.

Issues

  1. Slack community URL (inline on README.md:118) — https://reactrails.slack.com is the workspace URL; non-members cannot join from it. If the old invite link is dead, a new Slack shared-invite link or a redirect to GitHub Discussions would serve new contributors better.

  2. run_rspec:empty task no longer propagates COVERAGE=true (inline on react_on_rails_pro/rakelib/run_rspec.rake:25) — empty_spec.rb unconditionally loads SimpleCov, so running the task without a prior coverage-enabled suite will produce an empty merged report. Worth adding a COVERAGE=true pass-through or a note in the task desc.

Nits

  • simplecov is added without a version constraint in both Gemfile.development_dependencies files. Now that coveralls' ~> 0.16.1 pin is gone, a bundle update simplecov would pull in 0.22.x. The API is largely backward-compatible, but worth doing explicitly to validate the existing simplecov_helper.rb files against the newer API.
  • The pre-existing # TODO: This file is not used for CI comment at the top of react_on_rails_pro/rakelib/run_rspec.rake is more prominent now — good candidate for a follow-up.

- Pin `simplecov` to `~> 0.16.1` in both Gemfile.development_dependencies
  files so it follows the project's strict-version convention. Matches
  the version that was previously transitively pinned via coveralls and
  resolves consistently in all four lockfiles.
- README: clarify that the React + Rails Slack workspace URL only works
  for existing members and direct non-members to GitHub Discussions.
- run_rspec:empty: only invoke RSpec when COVERAGE=true; otherwise
  print a skip message. Without coverage enabled, the task would have
  loaded SimpleCov against an empty cache and produced a misleading
  report.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@justin808
Copy link
Copy Markdown
Member Author

Round 2 review triage (commit 42d2e0c)

I (Claude Code) walked the new claude[bot] feedback in #3204 (comment) and the two new inline comments. Code changes for the must-fix and the two optional refinements landed in 42d2e0c; the remaining nit is acknowledged below as a deliberate skip.

Addressed in code (42d2e0c)

  • simplecov version constraint (review nit) — Pinned to ~> 0.16.1 in both react_on_rails/Gemfile.development_dependencies:51 and react_on_rails_pro/Gemfile.development_dependencies:62, matching the version that was previously transitively pinned via coveralls. Re-resolved all four lockfiles (Gemfile.lock, react_on_rails/Gemfile.lock, react_on_rails/spec/dummy/Gemfile.lock, react_on_rails_pro/Gemfile.lock, react_on_rails_pro/spec/dummy/Gemfile.lock); simplecov stays at 0.16.1 across the board. Intentionally not pulling in 0.22.x in this PR — that upgrade should be its own change so the simplecov_helper.rb files can be validated against the newer API.
  • README Slack URL clarifier (inline #discussion_r3141547639) — Added a parenthetical note that https://reactrails.slack.com is for existing members and pointed non-members at GitHub Discussions (which is already listed two lines above). Did not touch the docs files (docs/oss/deployment/troubleshooting.md, docs/oss/getting-started/common-issues.md, etc.) — the bot itself flagged them as already consistent.
  • run_rspec:empty COVERAGE guard (inline #discussion_r3141547691) — Task now skips entirely when COVERAGE is unset and prints "Skipping run_rspec:empty (set COVERAGE=true to generate a SimpleCov report from cache)". This is a stronger guard than the suggested env-prefix variant: empty_spec.rb unconditionally requires SimpleCov, so the merged report is meaningless without coverage enabled.

Deliberately skipped

  • Pre-existing # TODO: This file is not used for CI at react_on_rails_pro/rakelib/run_rspec.rake:3 — the bot itself flagged this as a "good candidate for a follow-up". It pre-dates this PR and is out of scope for a Coveralls-removal change. Happy to track it as a separate cleanup issue.

Resolved both inline review threads. The simplecov and TODO items came in the review-summary body and have no inline thread to resolve.

@claude
Copy link
Copy Markdown
Contributor

claude Bot commented Apr 29, 2026

Code Review: Remove stale Coveralls integration

Overview

Clean, focused PR that removes the long-stale Coveralls integration across both OSS and Pro packages. The approach is correct — Coveralls was already pulling in SimpleCov as a transitive dependency, so switching to a direct SimpleCov dependency maintains identical behavior while eliminating the dead Coveralls layer. Low risk.


Positive observations

  • Dev tests generator improvement: Adding group: :test to the simplecov gem call in dev_tests_generator.rb is a genuine improvement over the old coveralls entry (which lacked a group). Correct placement.
  • Pro run_rspec:empty refactor: The task is now properly guarded — it only runs rspec spec/empty_spec.rb when COVERAGE=true is set, and skips otherwise with a clear message. Much better UX than the old approach of embedding an inline ternary.
  • Quote style cleanup in react_on_rails_pro/Gemfile.development_dependencies: Single-to-double quote standardization is a nice touch, aligns with RuboCop expectations.
  • Slack URL update: The invite.reactrails.com redirect was broken; the new reactrails.slack.com direct URL is correct. The added parenthetical note for non-members is the right approach since the old invite URL is gone.

Issues / observations

1. simplecov (~> 0.16.1) version constraint is stale (minor)

Both OSS and Pro Gemfiles now pin to ~> 0.16.1 (SimpleCov from 2019). This was inherited from coveralls, which had the same constraint. SimpleCov is now at 0.22.x with meaningful improvements (better branch coverage, HTML formatting, etc.). This PR is the right moment to consider relaxing to ~> 0.22, but it is out of scope if you want to keep the diff minimal. At minimum, worth a follow-up issue.

2. empty_spec.rb unconditionally requires SimpleCov (potential confusion)

Both react_on_rails/spec/empty_spec.rb and react_on_rails_pro/spec/empty_spec.rb do:

require "simplecov"
SimpleCov.command_name "empty_spec"

Unlike the simplecov_helper.rb files (which guard with if ENV["COVERAGE"] == "true"), this file loads SimpleCov unconditionally. With the Pro run_rspec:empty task now properly skipping when COVERAGE is unset, this is mostly harmless in practice, but a developer running rspec spec/empty_spec.rb directly without intent to generate a report will silently produce one. Not a blocker, but the guard could be added for consistency.

3. Pro run_rspec:run_rspec always depends on :empty, which now conditionally no-ops

The task chain run_rspec:run_rspec → [:gem, :dummy, :empty, :js_tests] will now print "Skipping run_rspec:empty..." every time rake run_rspec is invoked without COVERAGE=true. This is intentional and fine — just worth knowing the output will be slightly different in local dev.

4. No CHANGELOG entry

The removal of Coveralls (a visible tool change) and the update to the Slack link could merit a brief entry in the changelog under the next unreleased version. Not strictly required for tooling-only changes, but good practice given the project's existing CHANGELOG discipline.


Summary

Approve with minor notes. The change is correct and well-executed. The main suggestions — upgrading SimpleCov from 0.16.x and adding a COVERAGE guard to empty_spec.rb — are improvements but not blockers. The core cleanup is solid.

gem "capybara", "~> 3.40"
gem "capybara-screenshot"
gem "coveralls", require: false
gem "simplecov", "~> 0.16.1", require: false
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This version constraint (~> 0.16.1) is carried forward from the old coveralls gem requirement, which pinned to the same range. SimpleCov 0.16.x is from 2019; the current release line is 0.22.x, which adds branch coverage support and better HTML output. Consider loosening to ~> 0.22 either here or in a quick follow-up.

Suggested change
gem "simplecov", "~> 0.16.1", require: false
gem "simplecov", "~> 0.22", require: false


desc "run all tests"
task run_rspec: %i[gem dummy empty js_tests] do
puts "Completed all RSpec tests"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good improvement — the task now skips cleanly instead of running a no-op spec. One edge case: spec/empty_spec.rb itself does an unconditional require "simplecov" (no COVERAGE guard), so if a developer runs rspec spec/empty_spec.rb directly without COVERAGE=true, SimpleCov will still be loaded and may emit output. The task-level guard here is correct, but adding a matching guard in spec/empty_spec.rb would make direct invocations consistent too.

@justin808 justin808 merged commit d3b546e into main Apr 29, 2026
45 of 46 checks passed
@justin808 justin808 deleted the jg-conductor/issue-1692-badge branch April 29, 2026 08:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Fix Coveralls setup

1 participant